Skip to content

NO TICKET: move level_status from observation to field_activity & rename | make observation value non-nullable#169

Closed
jacob-a-brown wants to merge 2 commits into
stagingfrom
jab-observation-sample-field-transfer
Closed

NO TICKET: move level_status from observation to field_activity & rename | make observation value non-nullable#169
jacob-a-brown wants to merge 2 commits into
stagingfrom
jab-observation-sample-field-transfer

Conversation

@jacob-a-brown

@jacob-a-brown jacob-a-brown commented Oct 1, 2025

Copy link
Copy Markdown
Contributor

Why

This PR addresses the following problem / context:

  • an observation must have a value to be a valid observation, otherwise something occurred to prevent the observation from being observed.

How

Implementation summary - the following was changed / added / removed:

  • made the value field in observation non-nullable
  • move level_status from the observation table to the field_activity table and rename activity_status. This allows multiple activities to occur during an event, such as groundwater level and maintenance, but the status of one won't affect the status of the other.

Notes

Any special considerations, workarounds, or follow-up work to note?

  • Use bullet points here

@codecov-commenter

codecov-commenter commented Oct 1, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

Files with missing lines Coverage Δ
db/field.py 100.00% <100.00%> (ø)
db/observation.py 100.00% <ø> (ø)
schemas/field.py 100.00% <100.00%> (ø)
schemas/observation.py 98.33% <ø> (ø)
tests/conftest.py 98.81% <ø> (ø)
tests/test_observation.py 95.34% <ø> (ø)

@ksmuczynski

ksmuczynski commented Oct 1, 2025

Copy link
Copy Markdown
Contributor

Thinking more about lab data and value being nullable or not, I think it SHOULD be nullable. It's very common for labs to analyze samples, but the chemical is not present at a high enough concentration to be quantified. In these situations, the value is null, but there are lab qualifiers and reporting limits to provide further context.

ksmuczynski
ksmuczynski requested changes Oct 1, 2025
View reviewed changes
Comment thread db/observation.py
Comment on lines +59 to +60
nullable=False,
comment="The value of the observation. If a value could not be retrieved then an observation could not be made. The reason for the missing value should be documented in the correct table, like field_activity.",

@ksmuczynski ksmuczynski Oct 1, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking more about lab data and value being nullable or not, I think it SHOULD be nullable. It's very common for labs to analyze samples, but the chemical is not present at a high enough concentration to be quantified. In these situations, the value is null, but there are lab qualifiers and reporting limits to provide further context.

I agree NULL values can be a pain to work with sometimes, but no result is a result and the Observation table has additional fields to explain why.

@jacob-a-brown

jacob-a-brown commented Oct 1, 2025

Copy link
Copy Markdown
Contributor Author

Closing PR for more consideration, debate, and feedback from AMP before proceeding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ksmuczynski ksmuczynski ksmuczynski requested changes
@jirhiker jirhiker jirhiker approved these changes
@chasetmartin chasetmartin Awaiting requested review from chasetmartin

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jacob-a-brown @codecov-commenter @ksmuczynski @jirhiker